Skip to content

Readability refactors in sql_metadata and test fixture formatting - #667

Open
inquilabee wants to merge 1 commit into
macbre:masterfrom
inquilabee:shipgate-readability-pass
Open

Readability refactors in sql_metadata and test fixture formatting#667
inquilabee wants to merge 1 commit into
macbre:masterfrom
inquilabee:shipgate-readability-pass

Conversation

@inquilabee

@inquilabee inquilabee commented Jul 29, 2026

Copy link
Copy Markdown

I rely on sql-metadata for SQL parsing in analytics pipelines and wanted to contribute a small readability pass on the core package while running ShipGate for a broader quality picture.

Summary

ShipGate reported 72 format issues (mostly trailing whitespace in test SQL fixtures) and ~124 refactoring opportunities (strict pass) on a full-repo scan. Security scans and duplication checks were clean; the main themes are cyclomatic complexity in column_extractor / parser, large module sizes, and optional strict-refactor simplifications. This PR applies an auto-fix pass on the production package plus ruff formatting on a handful of test files where line-length and whitespace were noisy.

Hotspots and improvement notes below are scoped to Python (*.py) source.

What you are doing right

  • Security: No secrets or high-severity security patterns flagged (gitleaks/bandit/semgrep clean in scope).
  • Duplication: No significant clone duplication reported — jscpd looked clean.
  • Test discipline: 272 tests pass locally with 100% coverage on sql_metadata; mypy strict mode on the package is green.
  • Architecture: Clear v3 extractor composition (ColumnExtractor, TableExtractor, NestedResolver) with lazy caching on the Parser facade — well documented in ARCHITECTURE.md.
  • Tooling: Modern ruff + mypy + Poetry CI across Python 3.10–3.14 is solid.

What you could improve

  • Maintainability / complexity: _handle_cte and _handle_alias in column_extractor.py rank C on cyclomatic complexity (CC 11–12); values_dict in parser.py is similar — incremental helper extraction would help over time.
  • Module size: column_extractor.py (~900 lines), nested_resolver.py, and parser.py exceed typical module-size gates — consider splitting by clause type when the next feature lands.
  • Dead code: NoReturn import in query_type_extractor.py appears unused (vulture) — safe cleanup follow-up.
  • Refactoring (follow-up): ~100+ strict refactor hints remain in core modules — mostly assign-if-exp, walrus, and regex group indexing; many are optional style wins.
  • Test fixtures: Trailing whitespace in multi-line SQL strings in several test files triggers ruff W291 — make format on test/ would clear most of the 72 format findings.

Changes

Pass Files Fixes / rules Manual?
shipgate format --target sql_metadata 0 production tree already formatted no
shipgate refactor fix sql_metadata + strict hints 5 use-getitem-for-re-match-groups, use-named-expression, min-default, ternary cleanup partial
ruff format + ruff check --fix on test/ 8 W291/E501 line-wrap in SQL fixture strings, import merge no
Total 13 0
  • Use regex match indexing (match[1]) instead of .group(1) in parser.py and sql_cleaner.py.
  • Simplify ternary / walrus patterns in column_extractor.py, generalizator.py, and table_extractor.py.
  • Ruff-format test modules with long fixture lines and trailing whitespace.

We're testing ShipGate on real-world projects to learn whether it works well in practice. This review summary was generated from ShipGate check output and AI-assisted analysis. This is not spam, and our intention is not to waste your time or energy — we would genuinely welcome your feedback on the findings or approach. docs.

Full ShipGate findings: review gist

Simplify regex match indexing, ternary patterns, and min() default usage
in the core package; ruff-format test SQL fixtures with trailing whitespace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant